Skip to content

[HACK WEEK] Integrate periphery #15962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 22 commits into from
Aug 6, 2025
Merged

[HACK WEEK] Integrate periphery #15962

merged 22 commits into from
Aug 6, 2025

Conversation

@RafaelKayumov
Copy link
Contributor

@RafaelKayumov RafaelKayumov commented Jul 30, 2025 •
edited
Loading

Description

Integrate Periphery to run on Buildkite CI. Follows the approach introduced here: pbzQyC-3OH-p2

  • Implements the .periphery.yml setup.
  • Generates and uses Scripts/Periphery/periphery_baseline.json as filter list of existing legacy unused code issues.
  • Implements the Scripts/Periphery/setup-and-run-periphery.sh to prepare and use the Periphery installation
  • Implements the .buildkite/commands/run-periphery.sh as a Buildkite pipeline command.
  • Runs the setup-and-run-periphery.sh scan with --skip-build to reuse the build artifact from the build step
  • Extends the .buildkite/commands/build-for-testing.sh step to also upload the DerivedData/Index.noindex/DataStore/ data store index for the Periphery.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@RafaelKayumov RafaelKayumov added this to the 23.0 milestone Jul 30, 2025
@RafaelKayumov RafaelKayumov added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Jul 30, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 30, 2025 •
edited
Loading

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr15962-147232e
Version22.9
Bundle IDcom.automattic.alpha.woocommerce
Commit147232e
Installation URL4v922dds905a8
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@RafaelKayumov RafaelKayumov force-pushed the integrate-periphery branch 5 times, most recently from 09848fd to 1f4f250 Compare August 4, 2025 12:12
@itsmeichigo itsmeichigo mentioned this pull request Aug 5, 2025
Merged
1 task
@RafaelKayumov RafaelKayumov marked this pull request as ready for review August 5, 2025 11:28
@RafaelKayumov RafaelKayumov changed the title [HACK] Integrate periphery [HACK WEEK] Integrate periphery Aug 5, 2025
itsmeichigo
itsmeichigo reviewed Aug 6, 2025
View reviewed changes
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a novice when it comes to app infrastructure tasks so I'll refrain from reviewing this PR. I'm just curious where I can get the reports from Periphery after this setup? (pardon the dumb question)

@itsmeichigo itsmeichigo requested a review from a team August 6, 2025 01:59
@RafaelKayumov
Copy link
Contributor Author

RafaelKayumov commented Aug 6, 2025

I'm a novice when it comes to app infrastructure tasks so I'll refrain from reviewing this PR. I'm just curious where I can get the reports from Periphery after this setup? (pardon the dumb question)

You can go to the PR's Buildkite pipeline and check out the "Detect unused code" step. For example this is the one for this PR: https://buildkite.com/automattic/woocommerce-ios/builds/31502#019879ee-7dcd-453e-b5c2-1ed88921cf0f

If there are any unused code entries, the build will fail and the output will contain smth like:

woocommerce-ios/WooCommerce/Classes/Spotlight/SpotlightManager.swift:5:16: warning: Property 'unusedLetExample' is unused

AliSoftware
AliSoftware reviewed Aug 6, 2025
View reviewed changes
@dangermattic
Copy link
Collaborator

dangermattic commented Aug 6, 2025

1 Warning
⚠️ This PR is assigned to the milestone 23.0. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

AliSoftware
AliSoftware approved these changes Aug 6, 2025
View reviewed changes
Copy link
Contributor

@AliSoftware AliSoftware left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to unblock, just left a suggestions of adding 2 flags to the periphery scan to improve the output

[EDIT] Actually implemented those suggestions myself in a separate PR so that you have all the options (merge this base PR independently and iterate on the other one later, or merge the other one in this one first if you like it before merging all to trunk… up to you)

@AliSoftware AliSoftware mentioned this pull request Aug 6, 2025
Merged
@AliSoftware
Add Periphery errors as Buildkite annotation
5f1c6f0
@AliSoftware
Refactor setup-and-run-periphery.sh
55fa528 
Add --disable-update-check and --relative-results flags

DRY some code to avoid repetition of the commands to run scan
@AliSoftware
🚧 TO REVERT: Introduce violations on purpose
0203b2f
@RafaelKayumov
Copy link
Contributor Author

RafaelKayumov commented Aug 6, 2025

@AliSoftware Thx for contributions. I'll wait for your PR before merging.

@AliSoftware
Copy link
Contributor

AliSoftware commented Aug 6, 2025 •
edited
Loading

I'm just curious where I can get the reports from Periphery after this setup?

@itsmeichigo Quick update: following some improvements we made in #15984, the CI job now also posts a Buildkite annotation to report any Periphery warnings (example here) 🎉
So that's now another (and arguably nicer) way to see those 🙂 (though they're still in the job logs too if you prefer to look there)

@RafaelKayumov RafaelKayumov merged commit ad99355 into trunk Aug 6, 2025
14 checks passed
@RafaelKayumov RafaelKayumov deleted the integrate-periphery branch August 6, 2025 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AliSoftware AliSoftware AliSoftware approved these changes

@itsmeichigo itsmeichigo itsmeichigo left review comments

Assignees

No one assigned

Labels

category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc.

Projects

None yet

Milestone

23.0

Development

Successfully merging this pull request may close these issues.

6 participants

@RafaelKayumov @wpmobilebot @dangermattic @AliSoftware @itsmeichigo